Skip to content

Conversation

@efriedma-quic
Copy link
Collaborator

@efriedma-quic efriedma-quic commented Jun 20, 2025

443377a handled simple variable definitions, but it didn't handle uninitialized variables with a consteval constructor, and it didn't handle template instantiation.

Fixes #135281 .

@efriedma-quic efriedma-quic requested a review from cor3ntin June 20, 2025 02:24
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-coroutines

@llvm/pr-subscribers-clang

Author: Eli Friedman (efriedma-quic)

Changes

This is a simple extension of 443377a to also handle the implicit expressions created by default initialization. This usually doesn't matter, but it's relevant if the constructor stores "this" as a member.

Fixes #135281 .


Full diff: https://github.com/llvm/llvm-project/pull/144970.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaDecl.cpp (+6)
  • (modified) clang/test/SemaCXX/cxx2b-consteval-propagate.cpp (+18)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index bbd63372c168b..e10dc65897b8a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14423,6 +14423,9 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
         Var->getType().getAddressSpace() == LangAS::hlsl_input)
       return;
 
+    if (getLangOpts().CPlusPlus)
+      ActOnCXXEnterDeclInitializer(nullptr, Var);
+
     // C++03 [dcl.init]p9:
     //   If no initializer is specified for an object, and the
     //   object is of (possibly cv-qualified) non-POD class type (or
@@ -14458,6 +14461,9 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
     }
 
     CheckCompleteVariableDeclaration(Var);
+
+    if (getLangOpts().CPlusPlus)
+      ActOnCXXExitDeclInitializer(nullptr, Var);
   }
 }
 
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index dd5063cb29c5b..aa5d79af589ca 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -576,3 +576,21 @@ int f() {
   //expected-note@-2 {{read of non-const variable 'a' is not allowed in a constant expression}}
 }
 }
+
+#if __cplusplus >= 202302L
+namespace GH135281 {
+  struct B {
+    const void* p;
+    consteval B() : p{this} {}
+  };
+  B b;
+  B b2{};
+  B &&b3{};
+  void f() {
+    static B b4;
+    B b5; // expected-error {{call to consteval function 'GH135281::B::B' is not a constant expression}} \
+          // expected-note {{pointer to temporary is not a constant expression}} \
+          // expected-note {{temporary created here}}
+  }
+}
+#endif

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little suspicious of this change because right now we are only calling ActOnCXXExitDeclInitializer in ~InitializerScopeRAII() and now it seems like we need to sprinkle it in other places.

@efriedma-quic
Copy link
Collaborator Author

ActOnCXXExitDeclInitializer does three things:

  • Some scope handling which isn't relevant in this context.
  • Does some special handling for variables, to compute whether the variable's initializer is manifestly constant-evaluated.
  • Calls PopExpressionEvaluationContext

The special handling for variables is specifically relevant for variables... I don't see why we'd need it anywhere else? Except, hmm... I guess we need the same handling if we're instantiating a template, like:

struct B {
  const void* p;
  consteval B() : p{this} {}
};
template<typename T> T x{};
B* y = &x<B>;

But I can't think of anything else.

Any suggestions for how to refactor?

@efriedma-quic efriedma-quic force-pushed the consteval-uninit-var branch from b508252 to 75ae080 Compare July 1, 2025 23:56
@llvmbot llvmbot added the coroutines C++20 coroutines label Jul 1, 2025
@efriedma-quic efriedma-quic force-pushed the consteval-uninit-var branch from 75ae080 to b423545 Compare July 1, 2025 23:59
@efriedma-quic
Copy link
Collaborator Author

Came up with a different approach: I moved the check to PopExpressionEvaluationContext(). This required annotating the ExpressionEvaluationContextRecord to indicate we're handling a variable initializer.

443377a handled simple variables
definitions, but it didn't handle uninitialized variables with a
constexpr constructor, and it didn't handle template instantiation.

Fixes llvm#135281 .
@efriedma-quic efriedma-quic force-pushed the consteval-uninit-var branch from b423545 to b1f7402 Compare July 2, 2025 00:10
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a changelog entry? LGTM otherwiswe

@efriedma-quic efriedma-quic changed the title [clang] Handle consteval constructors with default initialization. [clang] Consistently handle consteval constructors for variables. Jul 2, 2025
@efriedma-quic efriedma-quic merged commit 6a99326 into llvm:main Jul 8, 2025
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category coroutines C++20 coroutines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

consteval constructor cannot store this inside object

5 participants